Fixed WFormWidget placeholder not updating on locale change #198
Conversation
src/Wt/WFormWidget.C
Outdated
| doJavaScript(jsRef() + ".wtObj" | ||
| ".setEmptyText(" + emptyText_.jsStringLiteral() + ");"); | ||
| if (isRendered()) { | ||
| if (env.agentIsIElt(10)) { |
There was a problem hiding this comment.
I think all IE-specific code should simply be removed.
There was a problem hiding this comment.
I removed all IE specific code from WFormWidget. Since WFormWidget::applyEmptyText contained only IE specific code, I removed that function and its calls as well. I decided to put these changes in a second commit so that they might be undone easily if they have some unforeseen consequences - without loosing the fix for the initial issue.
There was a problem hiding this comment.
I'm even wondering whether we really need any of the JavaScript that is attached to WFormWidget (defined in src/js/WFormWidget.js). I can't actually think of a scenario where we want placeholder text but there is no placeholder attribute on the HTML element to make it work. Same thing for the setTooltip() workaround: I doubt it's still necessary in 2022.
There was a problem hiding this comment.
I can look into it in the afternoon.
If I understand you correctly, you suggest to completely remove the attached JavaScript and all associated functionality in WFormWidget?
There was a problem hiding this comment.
Well, in the meantime I did already write the patch for it: https://gist.github.com/RockinRoel/40ba4fdf96f1318c0b022053cfc1d0da
I can't personally think of any regressions that would be caused by it. I think it should work in modern browsers, but any input or insights are appreciated.
There was a problem hiding this comment.
Nice. Thanks for the patch. I do agree that it should not cause any regressions.
Based on your patch I added some modifications. As you already said, there seems to be no scenario for placeholder texts on HTML elements without a placeholder attribute, so just <input> and <textarea>. I removed (or better hid) the placeholder functionality on all derived classes that are not (derived from) WLineEdit or WTextArea. Offering functionality that does nothing seems superfluous and potentially confusing. I know it is a potentially breaking API change, but since Wt 4.9 already does this due to removing jQuery, it might not be too bad.
I added a minor modification to WInPlaceEdit: It had a dedicated member variable for the placeholder, which seems unnecessary, since it used the functionality from the embedded WLineEdit anyways. Modified it to store the placeholder in WLineEdit now; saves some space.
I also ran a quick test: Works as intended for me, including the fix for the original issue. If you don't like the additional changes, you may just commit the changes from your patch, since they solve the initial issue.
skoehler47
force-pushed
the
fix-wt-04
branch
from
9e145d1 to
5d50ccf
Compare
fixed WFormWidget placeholder not updating on locale change
in classes derived from WFormWidget that are not suitable for placeholders
placeholder functionality; removed superfluous placeholder member
I found that the placeholder text of a
WLineEditwidget, that was set byWLineEdit::setPlaceholderText(WString::tr("..."))does not update when changing theWApplication's locale, as described in Issue 11156.This pull request contains a proposed fix for that issue.